Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nsc-events_3_39_update-user-dto-refactor #42

Merged
merged 14 commits into from
Nov 18, 2023

Conversation

CourtneyHoppus
Copy link
Contributor

@CourtneyHoppus CourtneyHoppus commented Nov 17, 2023

Resolves #39

I added a DTO for updating the user and refactored the controller and service to utilize this.
I removed the unused imports and the non-typed request parameters.
Removed redundant add User methods, the Auth module handles this.

When running npm install to be able to run the app with it's dependencies, it found a critical vulnerability:

Screenshot 2023-11-15 at 10 56 22 AM
Screenshot 2023-11-15 at 10 56 43 AM

I resolved this by running npm audit fix

Here is a document about connecting to a MongoDB Compass (local) instance. This is also the basic setup to connect to an Atlas (cloud) instance.

To test:

  • pull branch
  • run npm install
  • run locally
  • you must have a valid mongodb connection string and .env file set up
  • run tests on user in Postman
  • you must either
    • have a previously created user in your database and login through the /auth/login route
      OR:
    • create a new user using the /auth/signup route
  • using the token sent back when logging in or creating a user as your bearer token to test:
    • get all users at /users route with GET call
    • get user by id at /users/find/:id route with GET call
    • get user by email at /users/email/:email route with GET call
    • update a user at /users/update/:id route with PATCH call
    • delete a user at /users/remove/:id route with DELETE call

@CourtneyHoppus CourtneyHoppus added enhancement New feature or request Sprint 3 labels Nov 17, 2023
@CourtneyHoppus CourtneyHoppus self-assigned this Nov 17, 2023
@CourtneyHoppus CourtneyHoppus requested a review from a team as a code owner November 17, 2023 00:47
@CourtneyHoppus CourtneyHoppus requested review from brandondombrowsky and removed request for a team November 17, 2023 00:47
Copy link
Contributor

@brinkbrink brinkbrink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CourtneyHoppus Some checks are failing here. I also auto-updated your branch. The checks were failing before the update, too, FYI. Click the "details" link next to the failed check for more details. Some of them can be automatically fixed but I didn't go through and do that in case you wanted to check them out.

@CourtneyHoppus
Copy link
Contributor Author

CourtneyHoppus commented Nov 17, 2023

Thanks @brinkbrink. When referring to the automatic fix, is there a linter we are using that I can run on my branch or is it the third-party CodeFactor that wants permission to access GitHub?
EDIT: I saw that CodeFactor was running Prettier and that there was a file for that in the project so I used Prettier to lint the files it was complaining about.

Copy link
Contributor

@Robel-003 Robel-003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, nice job! All the api routes work as expected once I have signed up and signed in and used the provided token.
The update route returns helpful messages when updating a user. The delete api route doesn't seem to work though "/users/delete/:id", but this route works "/users/remove/:id"

Also, in the 4th bullet point under To test, I think it'd be helpful to add the connection link documentation Tin created.

@tinpham5614
Copy link
Contributor

tinpham5614 commented Nov 18, 2023

All endpoints are functional. It would be helpful to have a success message when removing a user.

@CourtneyHoppus
Copy link
Contributor Author

CourtneyHoppus commented Nov 18, 2023

@Robel-003 I added that link and updated the testing directions.
@brinkbrink I corrected the issues that were causing the checks to fail.
@tinpham5614 Happy to work on any additional back end issues that are found and created.

Copy link
Contributor

@Robel-003 Robel-003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great and it's working perfectly, thank you for making those changes! If you want to create issues for the comment Tin made, please feel free to do so. I'll go ahead and merge this into main

@Robel-003 Robel-003 merged commit e65061c into main Nov 18, 2023
@brinkbrink brinkbrink deleted the feature-39-update-user-dto-refactor-01 branch December 6, 2023 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Sprint 3
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Refactor: Add DTO to update a User
4 participants